Repository navigation
IB force history: record each run's last step; don't overwrite on restart - #1950
Open
sbryngelson wants to merge 5 commits into
Open
sbryngelson wants to merge 5 commits into
sbryngelson wants to merge 5 commits into
Conversation
…tart The record for step N is written at the start of step N, and the time loop exits before starting its last step, so no run ever recorded the force at t_step_stop. The next run skips its first step on purpose (that force is still the one from before the run), so every restart left exactly one step missing from the history. Write the last step's record after the loop. A resumed run also deleted and recreated D/ib_forces.dat, losing the earlier history. It now writes D/ib_forces_<t_step_start>.dat (D/ib_forces_n<n_start> .dat with cfl_dt), so the per-run files concatenate into a complete history. Co-Authored-By: Claude <noreply@anthropic.com>
Only the D/ib_forces.dat line changes: the old line is kept verbatim and the new last-step row(s) are appended. Regenerated with GNU 12.3 + MPI, Release on Frontier; every pre-existing value matches the committed NVHPC goldens under each test's tolerance. All other lines and the metadata are untouched. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes preserve existing sampling semantics, with reported restart and golden-test verification supporting correctness.
2 open findings
What changed in this PR
Updates MFC’s immersed-boundary force output to preserve history across restarts without changing the physics.
Changes:
- Writes the final force sample when allowed by the configured stride.
- Uses restart-specific filenames to preserve earlier history.
- Documents per-run output and extends force-history goldens.
| File | Description |
|---|---|
| tests/F200F862/golden.txt | Adds the final force record. |
| tests/E5B66084/golden.txt | Adds the final force record. |
| tests/B317404C/golden.txt | Adds the final force record. |
| src/simulation/p_main.fpp | Calls the history writer after time stepping. |
| src/simulation/m_data_output.fpp | Selects restart-specific history filenames. |
| docs/documentation/case.md | Describes endpoints and per-run history files. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ntees The final record still obeys ib_force_stride, and with cfl_dt the step counter restarts each run, so a gap-free join needs stride 1 there.
Enable ib_force_wrt in the particle-cloud restart cases. run_restart now appends the resumed run's ib_forces_<mid>.dat to the first run's ib_forces.dat, so the roundtrip checks that the first file survives and the two join into the straight run's history. Goldens gain the ib_forces.dat entry only.
Lines of Code
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1950 +/- ##
==========================================
- Coverage 61.79% 61.79% -0.01%
==========================================
Files 86 86
Lines 22773 22778 +5
Branches 3353 3355 +2
==========================================
+ Hits 14073 14075 +2
- Misses 6211 6213 +2
- Partials 2489 2490 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Chaining a run with
ib_force_wrtacross restarts loses IB force history in two ways.s_write_ib_force_history(t_step)runs at RK stage 1 of step N and records the force left by step N-1. The time loop inp_mainexits as soon ast_step == t_step_stop, so the record fort_step_stop(the force after the run's last step) is never written. The next run deliberately skips itst_step_start, because at that point the force is still the one from before the run (the existing comment explains this). So no run ever writes that step. Our exports showed exactly one missing record at each restart step, e.g. steps 12735, 25470 and 38205 in a three-chunk run.s_open_ib_force_historydeletes and recreatesD/ib_forces.daton every run. A restarted run therefore keeps only its own rows, and every earlier chunk's history is gone unless the user renamed the file in between.Fix.
p_maincallss_write_ib_force_history(t_step)once more after the time loop, which writes the run's last step. I did not write the resumed run's first step instead, because that would record the stale (zero) force the existing skip avoids.D/ib_forces_<t_step_start>.dat(D/ib_forces_n<n_start>.datwithcfl_dt) instead of overwritingD/ib_forces.dat. A fresh run still writesD/ib_forces.dat. Within each file the documented layout is unchanged: row 0 is that run's first recorded step, there is no header, and offsets are fixed.docs/documentation/case.mddescribes the per-run files and the new last row.Why not append into one file. That needs either absolute step-based rows, or a row base inferred from the existing file. Absolute rows leave NUL-filled holes whenever a run starts from a restart without the earlier history (the existing docs and comments avoid holes on purpose). An inferred base is silently misaligned after a chunk killed past its last restart dump. Separate files avoid both, and they still concatenate into the full history.
Impact on results and goldens
The physics is unchanged. Every run with
ib_force_wrtnow has one more row per body inD/ib_forces.dat. Ten goldens contain that file (135F548B, 49893269, 4BED9896, 5A22B45F, B317404C, C8AD6271, D6794F4C, E085CC5A, E5B66084, F200F862), and they are updated only on theD/ib_forces.datline:packtol.compareat each test's tolerance (1e-10, Examples 1e-3), the regenerated values for every field, including the existingib_forcesrows, match the committed NVHPC 25.11 goldens.ib_forcesline. The old line is a string prefix of the new one, every other line and everygolden-metadata.txtis untouched, and the change is one line per golden../mfc.sh test --generate --only <those 10>reproduces them.Verification
All runs were on a Frontier CPU compute node (GNU 12.3 + Cray MPICH, Release, 2 ranks). The case is a 2D circle moving at a prescribed velocity, with
ib_force_wrt.ib_forces.dat,ib_forces_20.dat)masterOn this branch the two files concatenated match the straight run's
ib_forces.dat: the same 40 times, and a maximum absolute difference of 2e-17 over all columns.With the updated goldens, on the same node:
ib_forcestestsmasterVariable count didn't match for D/ib_forces.dat(the missing last row)simulationcompiles with CCE 19 (CPU) and GNU 12.3. Precheck passes, apart from twotest_thermochemcases that fail on the Frontier login node because they compile with the system/usr/bin/gfortran(addressed by #1943).Contribution Policy
We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
This PR was prepared with the assistance of an AI tool (Claude Code). We hit this in chunked production runs on Frontier; the fix was exercised on the small cases above.
PR template credit: junegunn